feat(storage): Implement robust path validation and structured skip reporting#7546
feat(storage): Implement robust path validation and structured skip reporting#7546thiyaguk09 wants to merge 9 commits intogoogleapis:mainfrom
Conversation
thiyaguk09
commented
Mar 10, 2026
thiyaguk09
commented
- Adds protection against path traversal (../) using normalized path resolution.
- Prevents Windows-style drive letter injection while allowing GCS timestamps.
- Implements directory jail logic to ensure absolute-style paths are relative to destination.
- Preserves backward compatibility by returning an augmented DownloadResponse array.
- Automates recursive directory creation for validated nested files.
- Adds a comprehensive 13-scenario test suite for edge-case parity.
- Adds protection against path traversal (../) using normalized path resolution. - Prevents Windows-style drive letter injection while allowing GCS timestamps. - Implements directory jail logic to ensure absolute-style paths are relative to destination. - Preserves backward compatibility by returning an augmented DownloadResponse array. - Automates recursive directory creation for validated nested files. - Adds comprehensive 13-scenario test suite for edge-case parity.
product-auto-label
bot
added
the
api: storage
quirogas
added
the
priority: p2
- Implemented "jail" logic using path.resolve to prevent traversal. - Neutralized leading slashes in GCS object names. - Pre-allocated results array to maintain 1:1 input/output index parity. - Added automatic recursive directory creation for nested local paths. - Fixed prioritization of destination options in downloadManyFiles.
- Reordered security checks to catch illegal drive letters before path resolution. - Fixed SkipReason mismatch (Illegal Character vs Path Traversal) on Windows. - Ensured absolute Windows paths are neutralized before traversal validation.
…fety Isolated async loop variables to fix path leakage, decoupled prefix from destination logic, and added cross-platform traversal checks for both forward and backslashes.
Updated Parity Check tests with platform-conditional logic to handle OS-specific backslash resolution.
| if (options.stripPrefix) { | ||
| passThroughOptionsCopy.destination = file.name.replace(regex, ''); | ||
|
|
||
| const normalizedGcsName = file.name.replace(/^[\\/]+/, ''); |
There was a problem hiding this comment.
The stripPrefix operation should happen before this operation, right?
There was a problem hiding this comment.
Actually, no — By de-rooting first, we ensure the string starts exactly where the user expects, making the stripPrefix regex much more reliable.
There was a problem hiding this comment.
If object name is "/a/b/c.txt" and if user provides a stripPrefix: "/a/b", if you first remove the leading "/", the normalizedGcsName becomes "a/b/c.txt" after which the stripPrefix will not take any effect. Am I missing something here?
There was a problem hiding this comment.
Actually it is a valid point, and I agree with your assessment. However, shifting this to the beginning may introduce additional challenges. I will explore further options and follow up with you.
That was a great observation.
There was a problem hiding this comment.
Actually it's working as expected. I have updated the order of operations as suggested. The stripPrefix now runs against the raw GCS name first to ensure any leading slashes in the user's prefix match correctly. I then de-root the resulting string to ensure the final path is relative and stays within the destination 'jail' during path.resolve.
3 participants
